Conversation
gpunto
changed the title
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughRemoves per-user muting for 1:1 channels, limits swipe primary action to mute/unmute, deletes group-channel mute features, drops Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.kt (1)
101-143:⚠️ Potential issue | 🟠 MajorDon’t let
isMuteChannelVisiblehideMute Userin DMs.After removing group mute actions, this builder still wires
optionVisibility.isMuteChannelVisibleintobuildDmMuteUserAction. That means a caller can’t hide channel mute/unmute without also hiding the DM-sheetMute Useraction, which no longer matches the flag name/KDoc or the new channel-vs-user split in this PR. The same cleanup also leavesbuildDefaultChannelActions.isMutedlooking like a no-op parameter. Please finish the split by decoupling DM user-mute visibility from channel-mute visibility and dropping the dead argument while you’re here.Based on learnings, PRs against major version branches such as
v7may introduce breaking public API changes without compatibility overloads.Also applies to: 171-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.kt` around lines 101 - 143, The DM mute action visibility is incorrectly tied to optionVisibility.isMuteChannelVisible and the isMuted parameter is now unused; update buildDefaultChannelActions to remove the dead isMuted parameter, introduce/consume a DM-specific visibility flag (e.g. optionVisibility.isMuteUserVisible) and pass that to buildDmMuteUserAction via buildDmChannelActions (or add a new parameter to buildDmChannelActions/buildDmMuteUserAction) so hiding channel mute no longer hides the DM "Mute User" action; also update related KDoc and any call sites (and the other occurrence around lines 171-176) to use the DM-specific flag instead of isMuteChannelVisible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channels/list/DefaultChannelSwipeActions.kt`:
- Around line 93-108: The cached ChannelAction uses remember(channel.cid,
isMuted, canMute) so its label won't update on locale change; compute the
localized labels (e.g., val muteLabel =
resources.getString(R.string.stream_compose_swipe_action_mute) and val
unmuteLabel = resources.getString(R.string.stream_compose_swipe_action_unmute))
before calling remember and include those label strings in the remember key
(remember(channel.cid, isMuted, canMute, muteLabel, unmuteLabel)) so
MuteChannel/UnmuteChannel instances are recreated with updated localized text;
keep the existing handlerState/onAction logic and return the resolved action as
before.
---
Outside diff comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.kt`:
- Around line 101-143: The DM mute action visibility is incorrectly tied to
optionVisibility.isMuteChannelVisible and the isMuted parameter is now unused;
update buildDefaultChannelActions to remove the dead isMuted parameter,
introduce/consume a DM-specific visibility flag (e.g.
optionVisibility.isMuteUserVisible) and pass that to buildDmMuteUserAction via
buildDmChannelActions (or add a new parameter to
buildDmChannelActions/buildDmMuteUserAction) so hiding channel mute no longer
hides the DM "Mute User" action; also update related KDoc and any call sites
(and the other occurrence around lines 171-176) to use the DM-specific flag
instead of isMuteChannelVisible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 830cae27-743b-416a-94e5-5ab90364ac70
📒 Files selected for processing (8)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channels/list/DefaultChannelSwipeActions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptionItemVisibility.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.ktstream-chat-android-compose/src/main/res/values/strings.xmlstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.ktstream-chat-android-ui-common/api/stream-chat-android-ui-common.apistream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/channels/actions/ChannelAction.kt
💤 Files with no reviewable changes (4)
- stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.kt
- stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/channels/actions/ChannelAction.kt
- stream-chat-android-compose/src/main/res/values/strings.xml
- stream-chat-android-ui-common/api/stream-chat-android-ui-common.api
...c/main/java/io/getstream/chat/android/compose/ui/channels/list/DefaultChannelSwipeActions.kt
Show resolved
Hide resolved
gpunto
force-pushed
the
rm-archive-primary
branch
2 times, most recently
from
13dd62d to
8ed2de5
Compare
| ) : ChannelAction { | ||
| @DrawableRes | ||
| override val icon: Int = R.drawable.stream_ic_action_leave | ||
| override val requiredCapability: String = ChannelCapabilities.LEAVE_CHANNEL |
There was a problem hiding this comment.
LGTM! Just one small question: I find it a bit strange that this was introduced in the refactor, but not actually used. Is it possible that we were missing some requiredCapabilities check? Or was its usage naturally removed as we were polishing the UI?
There was a problem hiding this comment.
TBH I'm not sure, maybe @aleksandar-apostolov remembers?
There was a problem hiding this comment.
Btw, I don't know the original intent, but the code checks the capabilities before instantiating an action (e.g. in buildGroupChannelActions), so having requiredCapability in the action itself seems redundant. Checking out the original commit that introduced it, it's unused there too, so probably it was an initial API design iteration
gpunto
force-pushed
the
rm-archive-primary
branch
from
8ed2de5 to
f008b04
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stream-chat-android-compose/api/stream-chat-android-compose.api`:
- Around line 1201-1211: ChannelOptionItemVisibility's public API changed: its
constructor and copy(...) gained a seventh Boolean parameter isMuteUserVisible
(see <init>, copy and copy$default signatures), so add a migration note in
MIGRATION_TO_V7.md or CHANGELOG.md that describes the signature change (6 -> 7
params), lists the affected symbols (ChannelOptionItemVisibility constructor,
component1..component7, copy, copy$default), shows a before/after usage example,
and instructs consumers to add the new isMuteUserVisible Boolean (or use the
library's default behavior) when calling the constructor or copy to restore
previous behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a2e40ddd-d2a7-4f46-bdf4-4333f0be519d
📒 Files selected for processing (6)
stream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channels/list/DefaultChannelSwipeActions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptionItemVisibility.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.ktstream-chat-android-compose/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- stream-chat-android-compose/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/channels/list/DefaultChannelSwipeActions.kt
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/ChannelOptions.kt
gpunto
force-pushed
the
rm-archive-primary
branch
from
1c56852 to
4dddc2d
Compare
|
2 participants
Goal
Clean up channel actions:
Implementation
ChannelAction.requiredCapability(it was unused)ChannelOptionItemVisibilityas we don't have an archive screen in the SDKChannelOptionItemVisibility(e.g.isMuteChannelVisible)🎨 UI Changes
None
Testing
isMuteChannelVisible = falseinChannelOptionItemVisibility→ verify the mute swipe action is hiddenSummary by CodeRabbit